Thank you @LesnyRumcajs for catching this. You're seeing the result of first-timers (@TippyFlitsUK and me) in action :). It wasn't on the checklists we have that we can see. We'll get that noted so it doesn't get missed in the future.

The thing I want to confirm now is that there isn't other things we're missing before we need to cut another RC. We'll check notes @rjan90 left us and look at last network upgrade, but just was curious how you knew to do this...

Thanks again!

@LesnyRumcajs : Sorry not to know this myself... Obviously this change should get made, but I'm wondering if anything broke as a result of not having it. I'm asking for trying to gauge whether we need to rush out an RC2 right now. (We'll still work on getting this landed regardless.).

I also want to track down when this was done for last network upgrade to see if there's anything else we're missing.

(I noted in https://docs.google.com/document/d/1JedaSyUxbnujjxFUNU-bF8CwXtUP85HGEG9DYukvHL0/edit?tab=t.7nfpv13dea7c that we're missing steps about going from network name to code change)

We'll check notes @rjan90 left us and look at last network upgrade, but just was curious how you knew to do this...

I stumbled upon it because in Forest we do devnet interop tests that set upgrade heights and LOTUS_GOLDEN_WEEK_HEIGHT didn't work.

but I'm wondering if anything broke as a result of not having it

To my understanding, the impact is the return object of Filecoin.GetNetworkParams. I don't think anyone is relying on the new field introduced there except for parity tests, so it should be fine. Another thing is the aforementioned environmental variable; in Forest we'll have to use LOTUS_XX_HEIGHT until fixed, but it's not a huge deal.

Great - thanks for the context!

I'm also comparing to the last time @rjan90 did this: #12754 .

Anyways, this PR seems fine to me but if not urgent, I think we'll leave for @rjan90 on 2025-09-08 to review. We'll include it in the next RC/release of Lotus 1.34.0. I've added it to #13269 so it isn't missed.

@LesnyRumcajs : @TippyFlitsUK and I are going to target this against release/v1.34.0 for now. We'll work on this and update the conflicts accordingly.

@LesnyRumcajs : I am closing this in place of #13319 . I wanted to target the release branch, but I didn't have permissions to push my fix of the merge conflicts to your fork. Thanks for your help here!